Skip to content

perf(trace): capture page state only for commands that change it - #442

Open
Winify wants to merge 2 commits into
webdriverio:mainfrom
Winify:perf/capture-only-page-changes
Open

Winify wants to merge 2 commits into
webdriverio:mainfrom
Winify:perf/capture-only-page-changes

Conversation

@Winify

@Winify Winify commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What & why

In trace mode every adapter took a per-action capture (page source and screenshot, plus DOM/a11y on web) for read and wait commands as well as for actions. On iOS a capture is expensive, because WDA computes visible for every node. One reported suite made 187 getPageSource calls for 12 interactions and slowed from 3:10 to 7:10.

The capture gate has always been mapCommandToAction, the table that decides which commands render as timeline rows. 5c54ae4 (#247) added the reads (getText, is*, waitFor*, getTitle, …) to ACTION_MAP so they would render as rows. From then on each of them was also captured, including every poll of a waiting matcher. One table was answering two questions.

  • shared: METHOD_EFFECT classifies every trace method as changes or reads. ACTION_MAP is typed against it, so an unclassified entry fails to compile. changesPage(command) is the new capture gate.
  • service / selenium / nightwatch: capture only when changesPage. The first traced command of each test is also captured, so a test that opens with reads still has a frame. Read and wait rows keep rendering and inherit the latest earlier capture through claimAfter's existing latestAtOrBefore fallback.
  • Initial-frame retry: the first capture is retried once, on the next traced command, when it comes back without a screenshot. captureActionSnapshot reports a failed screenshot as a snapshot with no screenshot rather than as null. The retry is capped at one per test, so a driver that cannot take screenshots doesn't capture every read again. The rule is the same in all three adapters.
  • service: the end-of-test capture is stamped at the test's last page change rather than at its last row. Otherwise that action's row, and the reads after it, would resolve to the capture taken before it.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Polish (an improvement to an existing feature)
  • Breaking change (existing behavior changes for users)
  • Documentation
  • Internal (build, CI, dependencies, tooling)

Packages touched

  • shared (types and contracts)
  • core (framework-agnostic capture/reporting)
  • elements (published element/snapshot API — @wdio/elements)
  • service (WebdriverIO adapter)
  • nightwatch-devtools (Nightwatch adapter)
  • selenium-devtools (Selenium adapter)
  • selenium-devtools-py (Selenium Python adapter)
  • backend (server)
  • app (UI)
  • script (page-injected runtime)
  • trace (Trace mode) (tests only)

Notes for reviewers

Why isn't this in core? The gate is one call per adapter, but each adapter captures at a different point: the service before the command, Selenium and Nightwatch after it. The shared part is the classification, and that lives in shared.

Behavior change in the service. The service captures before a command, so an action's row now shows the page just before the next page-changing command, after any waits and reads in between. Before, it showed the page just before the first read that followed. Take click(A) → waitForDisplayed → getText → click(C): A's row used to show the moment after the click, possibly mid-transition, and now shows the settled state. The number of captures per page change is unchanged; only their timing moves. Selenium and Nightwatch capture right after each command, so they are unaffected.

Real runtime, upstream/main vs this branch, 3 runs each, DEVTOOLS_MODE=trace. "Rows differing" compares each row's resolved snapshot page text against the base run. These runs were measured before the retry fix, which only changes the screenshot-failure path.

captures wall time (mean) rows differing from base
WDIO mocha, Chrome 51–52 → 15 34.5 s → 34.9 s 0 / 16
Android native (Appium, emulator) 19 → 8 55.2 s → 45.9 s 0 / 17
Nightwatch (1 branch run) 17 → 10 17.8 s → 17.6 s 0 / 21

The Nightwatch wait after the submit click still resolves to /secure: it inherits the click's own capture, which is taken after the click. Wall time on Chrome doesn't move because captures are cheap there.

Checks: after the rebase onto upstream/main, pnpm build passes, pnpm test reports 2859 passed and 3 skipped, and pnpm lint reports 0 errors. New tests cover:

  • the gate in each adapter;
  • the initial-frame retry when the first screenshot fails, or when the capture is null;
  • the one-retry cap when screenshots always fail;
  • the stamp on the end-of-test capture;
  • read rows resolving to the latest earlier capture.

Not measured: iOS, the case that motivated this. A rerun of the reporting suite on this build would confirm the gain.

Known limits:

  • Nightwatch describe/it: the plugin's per-test hook fires once per module, so the first-command capture happens once per module.
  • browser.url() with no argument: this getter still counts as a page change.
  • Concurrent commands: in Selenium and Nightwatch the first capture isn't awaited. A traced command issued before it settles doesn't get the retry.
  • execute counts as a page change. That includes Appium mobile: reads, which only read.
  • Python adapter: it still captures after every command. Mirroring the table there, with a parity test, is a follow-up.

Screenshots / recordings

n/a (no UI change)

🤖 Generated with Claude Code

@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues remain.

Summary

This PR separates timeline rows from page captures. changesPage now gates captures across the three JavaScript adapters.

  • Trace mode captures page state when a command changes it.
  • The service ties a test’s final capture to its last page change.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Mapped command] --> B{Changes the page?}
  B -->|Yes| C[Capture page state]
  B -->|No| D{Initial capture needed?}
  D -->|Yes| C
  D -->|No| E[Reuse earlier capture]
  C --> F[Render timeline row]
  E --> F
Loading

Reviews (2) · Last reviewed commit: "fix(trace): retry the initial frame only..." · Reviewed by Greptile

Comment on lines 380 to +383
if (snap) {
upsertRichestSnapshot(ctx.actionSnapshots, snap)
} else if (initialFrame && !changesPage(cmd.command)) {
capturer.awaitingInitialFrame = true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failed screenshot stops later attempts

If the first screenshot fails, captureActionSnapshot can still return a truthy object with screenshot: undefined. This branch leaves awaitingInitialFrame false, so later reads never retry. FrameSnapshotIndex skips that capture, and Selenium has no final capture to recover it. A read-only test can therefore finish without frames for its rows.

Clear the initial-frame flag only after a usable screenshot arrives. Apply the same check in Nightwatch and the service.

Winify commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Review

The direction is right and the diff is small. Splitting "does this get a timeline row" from "does this get a capture" via METHOD_EFFECT / changesPage fixes the root cause, and typing ACTION_MAP against METHOD_EFFECT means an unclassified command fails to compile. I also checked that nested execute calls inside web reads (e.g. isDisplayed) don't bring captures back: the service only captures topLevelUserCommand.

I'd ask for one fix before merging and have one design question.

Must fix

1. The initial-frame retry never fires. Greptile's finding is correct, and it's slightly worse than it describes. captureActionSnapshot (packages/core/src/action-snapshot.ts:126-175) catches every error inside and returns an object even when the screenshot probe failed (screenshot: undefined). It only returns null from its outer catch, and nothing in that body realistically throws. So:

  • else if (initialFrame && !changesPage(...)) awaitingInitialFrame = true in Selenium (commandPostActions.ts) and Nightwatch (session.ts) never runs in practice.
  • In the service, if (snap) this.#awaitingInitialCapture = false clears the flag even when there's no screenshot.
  • The new test at selenium-devtools/tests/action-snapshot.test.ts:161 mocks coreCapture to return null, which the real code doesn't do. The test passes on a path that can't happen.
  • The result: a test that opens with reads and whose first screenshot fails gets no frame. trace-frame-snapshots.ts:66 skips captures without a screenshot.

Suggested fix: gate on snap?.screenshot in all three adapters, and have the tests mock a capture that returns no screenshot.

Should address

2. The service's action rows now show a later moment (not mentioned in the PR). The service captures before a command and stamps the capture with the timestamp of the last page change. Take click(A) → waitForDisplayed → getText → click(C):

  • Before: A's row was captured just before waitForDisplayed, so it showed the state right after the click (spinner, transition).
  • Now: A's row is captured just before click(C), so it shows the state after all the waits.

The capture count is the same (one per change); only when it's taken moves. Showing the settled state may well be what we want, but it is a user-visible change. The "0 rows differing" table compares page text on suites that probably have no async transitions, so it wouldn't catch this. Please either state it in the changeset or check it on a suite with a loading state. Selenium and Nightwatch capture right after the command, so the adapters now differ here as well.

3. A late result from one test can flip the next test's flag (Selenium/Nightwatch). The capture promise isn't awaited. If test N's first capture resolves after test N+1 has already taken its initial frame, the retry branch sets awaitingInitialFrame = true again, and the next read gets a capture it didn't need. The cost is one extra capture, but a per-test generation counter would close it cheaply. This only matters once #1 makes that branch reachable.

Nits / questions

  • Inconsistent retry rule. Selenium and Nightwatch retry only when the first command was a read (!changesPage). The service retries whenever the capture failed. Pick one rule.
  • execute counts as changes. That's safe in general, but on Appium most execute('mobile: …') calls are reads (mobile: activeAppInfo, getContexts, …), and iOS is the platform this PR is meant to help. Does the 187-capture suite use many of these? If so, a follow-up could classify by the first argument.
  • iOS isn't measured, as the PR says. The PR's main justification is the iOS slowdown. A single rerun of the suite that reported it, before merging, would confirm the gain and show how much of the remaining capture count comes from execute and url() with no argument.
  • The Python adapter now diverges from the JS adapters on capture behavior. It's acknowledged as a follow-up. Please open a tracking issue so it doesn't get lost.

Generated by Claude Code

Winify and others added 2 commits October 27, 2026 09:32
Trace mode took a per-action capture (page source + screenshot, plus DOM
on web) for reads and waits as well as actions, because the capture gate
was mapCommandToAction, the timeline-rendering table. 5c54ae4 added the
reads to that table so they would render as rows, which also made every
read, and every poll of a waiting matcher, a capture. On iOS a capture is
expensive; a reported suite made 187 page-source calls for 12
interactions and went from 3:10 to 7:10.

ACTION_MAP's methods are now classified in a typed METHOD_EFFECT table
(an unclassified method fails to compile) and changesPage() gates the
capture in all three JS adapters. Reads keep their rows and inherit the
latest earlier capture through claimAfter's fallback. The first traced
command of each test is still captured so a test opening with reads has
a frame. The WDIO service stamps its end-of-test capture at the test's
last page change, otherwise that action's row resolves to the capture
from before it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
captureActionSnapshot reports a failed screenshot probe as a snapshot without
a screenshot, not as null, so the retry keyed on a falsy result never fired
and a test that opened with reads could end up with no frame. The flag now
stays armed until a capture carries a screenshot, under one rule in all three
adapters, and is retried at most once per test so a driver that cannot take
screenshots does not capture every read again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Winify
Winify force-pushed the perf/capture-only-page-changes branch from b012d0c to 14e1b41 Compare October 27, 2026 08:35

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant